Repository navigation
Conversation
There was a problem hiding this comment.
@lxy-9602, I left comments concerning fragmented-selection performance and the documentation of the built-in hash index.
There was a problem hiding this comment.
@lxy-9602, thanks for updates. I left two comments for a compilation failure after the merge from main, and repeated IN-predicate initialization for sparse fragmented selections. PTAL.
| Result<std::optional<RoaringBitmap64>> VisitFullTextMatch( | ||
| const std::shared_ptr<FullTextMatch>& full_text_match) override; |
There was a problem hiding this comment.
Remove the obsolete full-text visitor override. In the current merged tree, FunctionVisitor no longer declares VisitFullTextMatch or forward-declares FullTextMatch. Consequently, including this header alone fails with use of undeclared identifier 'FullTextMatch'; including paimon/predicate/full_text_match.h first still fails with only virtual member functions can be marked 'override'. Both failures reproduce with c++ -std=c++17 -fsyntax-only -x c++ -I include -I src -I /opt/homebrew/include - and the corresponding include directives on stdin. The clang CI job reports the undeclared-type error as well.
This blocks the ordinary core build regardless of whether full-text support is enabled, since realtime_index_reader.cpp is compiled unconditionally and this header is exported through paimon/api.h. Please remove the stale declaration and its implementation to match the current scalar visitor interface.
| // At 25% selectivity or above, evaluate the batch once to avoid repeatedly initializing | ||
| // predicate lookup state for fragmented selections. Only selected rows may enter the result. | ||
| if (selection.Cardinality() >= array->length() * kFullBatchSelectivityThreshold) { |
There was a problem hiding this comment.
Handle fragmented selections below the 25% cutoff as well. This cutoff fixes the dense case from the earlier review, but selecting every eighth row of an 8,192-row batch still takes the slice loop and calls PredicateFilter::Test() 1,024 times. Each call rebuilds the IN value-set array and initializes Arrow's lookup state, so ordinary file-index/DV reads can still regress severely compared with the previous single batch evaluation.
An independent C++ reproduction with local Arrow 19.0.1, 1,024 IN values, and that 12.5% selection took about 0.032 ms per batch for whole-batch evaluation versus 11.2 ms for single-row slices, with identical selected results. Even the new test's stride-64 case took about 0.032 ms versus 1.36 ms. These are kernel-level measurements, not Paimon Arrow 17 benchmarks, and reuse the value-set array rather than rebuilding it as this path does.
Please gather fragmented candidates for one predicate call, reuse prepared lookup state, or account for the number of runs and initialization cost when choosing the path. The new correctness test alone cannot detect this regression.
Purpose
Linked issue: #158
Add predicate indexes for realtime append-only and deduplicate reads. Queries can use indexed key and value fields to select candidate offsets before reading and filtering rows.
realtime.index.<plugin>.columns.[disk snapshot committed end, pinned memory end)and combine candidates with the pinned deletion vector.Indexes remain in memory when segment data spills and are retained by pinned read views until released.
Tests
Added or extended coverage for:
API and Format
RealtimeIndex,RealtimeIndexReader, andRealtimeIndexFactoryinterfaces.offset_rangetoRealtimeQueryContext; callers must supply the bounds associated with the pinned memory view and selected disk snapshot.Documentation
Introduce the optional configuration:
realtime.index.hash.columns=id,payloadGenerative AI tooling
Generated-by: OpenAI Codex (GPT-6)